GHA: Use SHA from PR HEAD commit for pkg name #1124
Conversation
By default GHA provides SHA of the merge commit even for unmerged PRs. This made matching build artifacts to the actual commit impossible.
|
I see your point about the merge commit being necessary. As someone who believes in vigorous rebasing, I wasn't thinking about the PR commit and the resulting merge having different trees. And, I also see your point about not labeling an artifact to be about the tip commit on a PR when it is the merge that is built. The problem is that this makes it hard to tie the artifact back to the sources it came from. Perhaps put "unmerged-pr-" or something in the artifact name, so we at least know it's off the rails? That's not strictly necessary, as the sha1 won't be in the repo, but it's a clue that it's a testing artifact vs one from a longer-term branch. |
tleedjarv
force-pushed
the
gha-gitsha
branch
from
60fdc70 to
979aeca
Compare
|
Added a commit for testing out the "unmerged-pr-" idea. It's certainly ugly but it kind of works, I think. |
|
I checked an artifact and see I am inclined to hit merge if you think it's ready. |
|
I guess so. Might just as well merge. |
I'm opening this PR for discussion (see #1123 (comment)) but I'm not sure it should be merged, as there are some caveats.
The CI script still builds the merge commit. It is possible to change this and build the PR HEAD, but I'm not sure it is a good idea. The point of CI is to run the build tests before merging and the PR HEAD might be way off from the actual merge result (if it hasn't been rebased recently, for example).
With CI building the merge commit (which I think is a good idea), is it actually a good idea to name the build artifacts with the SHA from PR HEAD? If the PR was not rebased recently then the build artifacts are not based off that HEAD, strictly speaking.
(Note that all of this discussion only matters for open PRs; CI builds on merges, direct pushes and tagging are unaffected.)